New LogRecord protobuf BLE logging - #627
Conversation
| log_record = mesh_pb2.LogRecord() | ||
| try: | ||
| log_record.ParseFromString(bytes(b)) | ||
| log_record.message = log_record.message.replace("\n", "") |
There was a problem hiding this comment.
What about instead of this we not include the new line on the sending side? Looks great otherwise.
|
I think that eventually here we should do something like #607 is doing in that we should publish a pubsub message for log lines. That PR (WIP, of course) only passes along the log line and not the whole I also have a question, which is whether any actual firmware releases went out with the now-legacy logging characteristic. We could possibly exclude it if there weren't any, but I haven't been watching that closely. Other than my question and the suggestion from geeksville about removing newlines on the send side, I think this is good for now, and we can augment it with the pubsub once we've got something on both the BLE and the streaming interface sides. |
|
Sounds good. I'm moving the newline handling to the device |
|
I agree about pubsubish cleanup of the Log system on the device side.
Because it really never should have been in RediectablePrint and has grown
quite a bit. I especially want to fix the thing Andre mentioned about
logging also working over the TCP endpoint.
I have a PR half done for this. I was hoping to send it in before I left
town for a two day camping trip. I'll be back tonight and try to send it
ASAP.
Also yeah: if we haven't done an official release with the initial logging
over BLE feature hopefully we only need to support both variants briefly.
Because it costs an extra 512 byte buffer at the BLE level.
(Sent from a phone - please ignore typos)
…On Mon, Jul 1, 2024, 20:10 Ian McEwen ***@***.***> wrote:
I think that eventually here we should do something like #607
<#607> is doing in that we
should publish a pubsub message for log lines. That PR (WIP, of course)
only passes along the log line and not the whole LogRecord, though.
I also have a question, which is whether any actual firmware releases went
out with the now-legacy logging characteristic. We could possibly exclude
it if there weren't any, but I haven't been watching that closely.
Other than my question and the suggestion from geeksville about removing
newlines on the send side, I think this is good for now, and we can augment
it with the pubsub once we've got something on both the BLE and the
streaming interface sides.
—
Reply to this email directly, view it on GitHub
<#627 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABXB2OZLPRBSKNZZH2H3F3ZKIK37AVCNFSM6AAAAABKGNHXTWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMBRG44DEMZTGM>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
3 participants
Kissing cousins ticket for meshtastic/firmware#4220